Fixes #15206: Handling keept_output and asynchronous parameters in the remote-run API #2302
Conversation
| @@ -65,6 +65,20 @@ impl RemoteRun { | |||
| }) | |||
| } | |||
|
|
|||
| pub fn reply_keep_output(&self, job_config: Arc<JobConfig>) -> impl warp::reply::Reply | |||
| { | |||
| if self.run_parameters.keep_output == true | |||
There was a problem hiding this comment.
no need to explicitely compare to true, cargo clippy would likely mention it
|
Commit modified |
1 similar comment
|
Commit modified |
| @@ -65,32 +65,66 @@ impl RemoteRun { | |||
| }) | |||
| } | |||
|
|
|||
| pub fn target_to_vector(&self, job_config: Arc<JobConfig>) -> Vec<String> { | |||
There was a problem hiding this comment.
This should be a method of RemoteRunTarget, called get_connected_nodes or something like this.
There was a problem hiding this comment.
Vector is a technical implementation detail, you should strive to use applicative business names as much as possible to keep things readable
| return Err(warp::reject::custom( | ||
| "keep_output and asynchronous cannot be true simultaneously", | ||
| )); | ||
| } | ||
| if self.target == RemoteRunTarget::All { | ||
| info!("conditions OK"); |
There was a problem hiding this comment.
this is probably useless, if there is no error we can assume parameters were correct
|
Commit modified |
1 similar comment
|
Commit modified |
| @@ -221,8 +250,33 @@ impl RunParameters { | |||
| .map_err(|e| panic!("error while running child: {}", e)); | |||
|
|
|||
| tokio::spawn(child_future); | |||
| //thread::sleep(time::Duration::from_millis(500)); | |||
| @@ -47,4 +47,36 @@ mod tests { | |||
|
|
|||
| assert_eq!(res.unwrap().text().unwrap(), "Unhandled rejection: Invalid agent Condition : Invalid agent Condition : Wrong condition: \'clas~1\', it should match ^[a-zA-Z0-9][a-zA-Z0-9_]*$".to_string()); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn integration() { | |||
There was a problem hiding this comment.
Could you give it a more descriptive name?
|
Commit modified |
|
PR rebased |
relay/sources/relayd/api_test.txt
Outdated
| @@ -0,0 +1 @@ | |||
| remote run -D class2,class3 -H server.rudder.local | |||
There was a problem hiding this comment.
This is the result of the test, it should not be included.
There was a problem hiding this comment.
Furthermore, temporary files created by tests should go into target/tmp
| @@ -0,0 +1 @@ | |||
| hello moto la | |||
|
Commit modified |
1 similar comment
|
Commit modified |
| .form(¶ms) | ||
| .send(); | ||
|
|
||
| let data = std::fs::read_to_string("./api_test.txt").expect("Unable to read file"); |
|
Commit modified |
2 similar comments
|
Commit modified |
|
Commit modified |
|
Commit modified |
|
OK, merging this PR |
https://issues.rudder.io/issues/15206